Skip to content

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Jul 30, 2025

  • tools/llvm-objcopy/MachO/update-section-object.test was failing on Windows since the input file (macho_sections.s) might be checked out with the wrong line ending, resulting in difference in the size of sections being checked.
  • Removed the check for Windows in AArch64Arm64ECCallLowering: when llc is run without an explicit target, the module's target triple is unknown so this assert fires.
  • Expect llvm/test/CodeGen/Generic/allow-check.ll to fail for Arm64EC: Global ISel is not supported.

@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Paoliello (dpaoliello)

Changes
  • tools/llvm-objcopy/MachO/update-section-object.test was failing on Windows since the input file (macho_sections.s) might be checked out with the wrong line ending, resulting in difference in the size of sections being checked.
  • The Arm64EC mangler would crash if an empty name was passed in. Trying to mangle to # resulted in downstream errors (LLVM permits empty names, but not referring to an empty name?), so added an explicit error.
  • Removed the check for Windows in AArch64Arm64ECCallLowering: when llc is run without an explicit target, the module's target triple is unknown so this assert fires.
  • In AArch64Arm64ECCallLowering, don't set COMDATs for functions that are declarations for the linker. These are not permitted to have COMDATs.
  • Expect llvm/test/CodeGen/Generic/allow-check.ll to fail for Arm64EC: Global ISel is not supported.
  • Don't use an empty function name in llvm/test/CodeGen/Generic/vector-constantexpr.ll, that's not what we're testing and it's not supported on Arm64EC.

Full diff: https://github.com/llvm/llvm-project/pull/151409.diff

5 Files Affected:

  • (modified) llvm/.gitattributes (+1)
  • (modified) llvm/lib/IR/Mangler.cpp (+3)
  • (modified) llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp (+1-4)
  • (modified) llvm/test/CodeGen/Generic/allow-check.ll (+1)
  • (modified) llvm/test/CodeGen/Generic/vector-constantexpr.ll (+1-1)
diff --git a/llvm/.gitattributes b/llvm/.gitattributes
index fc3afb28a8d52..9f4ed8a24ecd0 100644
--- a/llvm/.gitattributes
+++ b/llvm/.gitattributes
@@ -32,3 +32,4 @@ test/tools/split-file/basic.test text eol=lf
 test/tools/split-file/Inputs/basic-*.txt eol=lf
 test/tools/split-file/basic.crlf.test text eol=crlf
 test/tools/split-file/Inputs/basic-*.crlf eol=crlf
+test/tools/llvm-objcopy/MachO/Inputs/macho_sections.s text eol=lf
diff --git a/llvm/lib/IR/Mangler.cpp b/llvm/lib/IR/Mangler.cpp
index 010bd15e256dc..894cd2dab599a 100644
--- a/llvm/lib/IR/Mangler.cpp
+++ b/llvm/lib/IR/Mangler.cpp
@@ -292,6 +292,9 @@ void llvm::emitLinkerFlagsForUsedCOFF(raw_ostream &OS, const GlobalValue *GV,
 }
 
 std::optional<std::string> llvm::getArm64ECMangledFunctionName(StringRef Name) {
+  if (Name.empty())
+    reportFatalUsageError("ARM64EC does not support empty function names");
+
   if (Name[0] != '?') {
     // For non-C++ symbols, prefix the name with "#" unless it's already
     // mangled.
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 509cbb092705d..5b5c8aa1d6503 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -727,9 +727,6 @@ AArch64Arm64ECCallLowering::buildPatchableThunk(GlobalAlias *UnmangledAlias,
 
 // Lower an indirect call with inline code.
 void AArch64Arm64ECCallLowering::lowerCall(CallBase *CB) {
-  assert(CB->getModule()->getTargetTriple().isOSWindows() &&
-         "Only applicable for Windows targets");
-
   IRBuilder<> B(CB);
   Value *CalledOperand = CB->getCalledOperand();
 
@@ -872,7 +869,7 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
     if (!F.isDeclaration() && (!F.hasLocalLinkage() || F.hasAddressTaken()) &&
         F.getCallingConv() != CallingConv::ARM64EC_Thunk_Native &&
         F.getCallingConv() != CallingConv::ARM64EC_Thunk_X64) {
-      if (!F.hasComdat())
+      if (!F.hasComdat() && !F.isDeclarationForLinker())
         F.setComdat(Mod.getOrInsertComdat(F.getName()));
       ThunkMapping.push_back(
           {&F, buildEntryThunk(&F), Arm64ECThunkType::Entry});
diff --git a/llvm/test/CodeGen/Generic/allow-check.ll b/llvm/test/CodeGen/Generic/allow-check.ll
index 148ee811ea806..97719a7af6229 100644
--- a/llvm/test/CodeGen/Generic/allow-check.ll
+++ b/llvm/test/CodeGen/Generic/allow-check.ll
@@ -6,6 +6,7 @@
 ; XFAIL: target=nvptx{{.*}}
 ; XFAIL: target=sparc{{.*}}
 ; XFAIL: target=hexagon-{{.*}}
+; XFAIL: target=arm64ec-{{.*}}
 
 ; RUN: llc < %s -O3 -global-isel=0 -fast-isel=0
 ; RUN: llc < %s -O3 -global-isel=1 -fast-isel=0
diff --git a/llvm/test/CodeGen/Generic/vector-constantexpr.ll b/llvm/test/CodeGen/Generic/vector-constantexpr.ll
index 416367fa3b52a..6e6a0c18d92d5 100644
--- a/llvm/test/CodeGen/Generic/vector-constantexpr.ll
+++ b/llvm/test/CodeGen/Generic/vector-constantexpr.ll
@@ -1,6 +1,6 @@
 ; RUN: llc < %s
 	
-define void @""(ptr %inregs, ptr %outregs) {
+define void @test(ptr %inregs, ptr %outregs) {
         %a_addr.i = alloca <4 x float>          ; <ptr> [#uses=1]
         store <4 x float> < float undef, float undef, float undef, float undef >, ptr %a_addr.i
         ret void

@efriedma-quic
Copy link
Collaborator

LLVM supports unnamed global variables and function; if such a function actually needs a name, we invent one later. (See Mangler::getNameWithPrefix.) Not really that useful, or frequently used, but it's there. I guess we can make AArch64Arm64ECCallLowering introduce names if they're necessary. If you're not going to fix it, please open a bug.

Please split out the available_externally fix, with an appropriate testcase.

@dpaoliello dpaoliello marked this pull request as draft July 30, 2025 23:46
dpaoliello added a commit that referenced this pull request Aug 1, 2025
While testing Arm64EC, I observed that LLVM crashes when an
`available_externally` function is used as it tries to place it in a
COMDAT, which is not permitted by the verifier.

This the fix from #151409 plus a dedicated test.
dpaoliello added a commit that referenced this pull request Aug 4, 2025
While testing Arm64EC, I observed that LLVM crashes when an empty
function name is used. My original fix in #151409 was to raise an error,
but this change now handles the empty name via
`Mangler::getNameWithPrefix` (which assigns a name to the function).

To get this working, I had to create the `Mangler` in
`TargetLoweringObjectFile` early so it would be available to Arm64EC's
lowering. There's no reason why `Mangler` is only created when
`Initialize` is called (or re-created if it exists), and so I moved
creation to the constructor and switched the raw pointer for a
`unique_ptr` to avoid the explicit `delete` in the destructor.
@dpaoliello dpaoliello marked this pull request as ready for review August 4, 2025 23:59
@dpaoliello
Copy link
Contributor Author

LLVM supports unnamed global variables and function; if such a function actually needs a name, we invent one later. (See Mangler::getNameWithPrefix.) Not really that useful, or frequently used, but it's there. I guess we can make AArch64Arm64ECCallLowering introduce names if they're necessary. If you're not going to fix it, please open a bug.

Please split out the available_externally fix, with an appropriate testcase.

Both of these have been fixed in other PRs, so this PR is ready to land now.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dpaoliello dpaoliello merged commit 07da480 into llvm:main Aug 5, 2025
11 checks passed
@dpaoliello dpaoliello deleted the arm64ecfixes2clean branch August 5, 2025 21:54
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 5, 2025

LLVM Buildbot has detected a new failure on builder mlir-nvidia-gcc7 running on mlir-nvidia while building llvm at step 7 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/16594

Here is the relevant piece of the build log for the reference
Step 7 (test-build-check-mlir-build-only-check-mlir) failure: test (failure)
******************** TEST 'MLIR :: Integration/GPU/CUDA/async.mlir' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -gpu-kernel-outlining  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -pass-pipeline='builtin.module(gpu.module(strip-debuginfo,convert-gpu-to-nvvm),nvvm-attach-target)'  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -gpu-async-region -gpu-to-llvm -reconcile-unrealized-casts -gpu-module-to-binary="format=fatbin"  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -async-to-async-runtime -async-runtime-ref-counting  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -convert-async-to-llvm -convert-func-to-llvm -convert-arith-to-llvm -convert-cf-to-llvm -reconcile-unrealized-casts  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-runner    --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_cuda_runtime.so    --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_async_runtime.so    --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_runner_utils.so    --entry-point-result=void -O0  | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/FileCheck /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -gpu-kernel-outlining
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt '-pass-pipeline=builtin.module(gpu.module(strip-debuginfo,convert-gpu-to-nvvm),nvvm-attach-target)'
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -gpu-async-region -gpu-to-llvm -reconcile-unrealized-casts -gpu-module-to-binary=format=fatbin
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -async-to-async-runtime -async-runtime-ref-counting
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-opt -convert-async-to-llvm -convert-func-to-llvm -convert-arith-to-llvm -convert-cf-to-llvm -reconcile-unrealized-casts
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/mlir-runner --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_cuda_runtime.so --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_async_runtime.so --shared-libs=/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/lib/libmlir_runner_utils.so --entry-point-result=void -O0
# .---command stderr------------
# | 'cuStreamWaitEvent(stream, event, 0)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventDestroy(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuStreamWaitEvent(stream, event, 0)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventDestroy(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuStreamWaitEvent(stream, event, 0)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuStreamWaitEvent(stream, event, 0)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventDestroy(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventDestroy(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventSynchronize(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# | 'cuEventDestroy(event)' failed with 'CUDA_ERROR_CONTEXT_IS_DESTROYED'
# `-----------------------------
# executed command: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.obj/bin/FileCheck /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir
# .---command stderr------------
# | /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir:68:12: error: CHECK: expected string not found in input
# |  // CHECK: [84, 84]
# |            ^
# | <stdin>:1:1: note: scanning from here
# | Unranked Memref base@ = 0x55a6cb912c40 rank = 1 offset = 0 sizes = [2] strides = [1] data = 
# | ^
# | <stdin>:2:1: note: possible intended match here
# | [42, 42]
# | ^
# | 
# | Input file: <stdin>
# | Check file: /vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/mlir/test/Integration/GPU/CUDA/async.mlir
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |             1: Unranked Memref base@ = 0x55a6cb912c40 rank = 1 offset = 0 sizes = [2] strides = [1] data =  
# | check:68'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
# |             2: [42, 42] 
# | check:68'0     ~~~~~~~~~
# | check:68'1     ?         possible intended match
...

@dpaoliello
Copy link
Contributor Author

LLVM Buildbot has detected a new failure on builder mlir-nvidia-gcc7 running on mlir-nvidia while building llvm at step 7 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/16594

Here is the relevant piece of the build log for the reference

Seems unrelated. This change only affected codegen for Arm64EC and modified two specific tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants